Don't use deprecated methods in resolve_with()#95
Merged
sigmavirus24 merged 2 commits intopython-hyper:mainfrom Feb 22, 2023
Merged
Don't use deprecated methods in resolve_with()#95sigmavirus24 merged 2 commits intopython-hyper:mainfrom
sigmavirus24 merged 2 commits intopython-hyper:mainfrom
Conversation
resolve_with(), which is not deprecated, was using the deprecated is_valid() method. Replaced with a lazily instantiated validator object.
src/rfc3986/_mixin.py
Outdated
| def _match_subauthority(self): | ||
| return misc.SUBAUTHORITY_MATCHER.match(self.authority) | ||
|
|
||
| def _get_base_uri_validator(self): |
Collaborator
There was a problem hiding this comment.
I'd rather this be a property that looks like:
@property
def _validator(self):
v = getattr(self, '_cached_validator', None)
if v is not None:
return v
self._cached_validator = validators.Validator().require_presence_of("scheme")
return self._cached_validatorAnd then used as
self._validator.validate(base_uri)
Contributor
Author
There was a problem hiding this comment.
@sigmavirus24 thanks, I've updated this and added a test needed to cover the case of the validator already existing with this code arrangement.
This required adding a test to cover the line where the validator has already been created.
sigmavirus24
approved these changes
Feb 22, 2023
Collaborator
|
Thank you @handrews ! |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #86: resolve_with(), which is not deprecated, was using the deprecated is_valid() method. Replaced with a lazily instantiated validator object.
I'm happy to change this approach, it's just what first came to mind. The linter was unhappy with the complexity of
resolve_with()unless I moved theself._base_uri_validatorinstantiation check to its own method.End of the tox output (run with 3.7.16, 3.8.13, 3.9.6, 3.10.10, 3.11.2, 3.12.0a5), run with this branch on top of the branch for PR #94 to avoid superfluous reformatting output:
Note that
_mixin.pywas previously 112 statements, now 120.